feat: add elicitation callback support to MCP servers #2373
Conversation
PR Change SummaryAdded support for elicitation callbacks in MCP servers, allowing user interaction during tool execution.
Modified Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
There was a problem hiding this comment.
@yamanahlawat The code and test looks good but the examples are wrong 😅
There was a problem hiding this comment.
@yamanahlawat Still some issues with the examples, did you verify they actually work? They smell a bit AI generated to me with too little human oversight 😅 I realize this isn't the feature you were initially trying to implement, but unfortunately we can't get to that one until this feature it builds on is solid so I'd appreciate a bit more love for the examples here
Guilty as charged. Thanks for the feedback. Here is my updated love for the docs, let me know if any other changes are required. |
|
|
@Kludex Can you please review this as our resident MCP expert? I'm thinking it would be more proper to add our own types instead of directly using those from |
…elicitation-callback
I think it would be easier if we use their types. |
|
@DouweM Can we merge this? or i'll add the server logic in this PR? |
|
|
||
|
|
||
| @mcp.tool() | ||
| async def use_elicitation(ctx: Context[ServerSessionT, LifespanContextT, RequestT], question: str) -> str: |
There was a problem hiding this comment.
This is not the correct type.
It's probably this:
| async def use_elicitation(ctx: Context[ServerSessionT, LifespanContextT, RequestT], question: str) -> str: | |
| async def use_elicitation(ctx: Context[ServerSession, None], question: str) -> str: |
There was a problem hiding this comment.
i noticed the other mcp test functions (echo_deps, use_sampling) all use Context[ServerSessionT, LifespanContextT, RequestT] as types. for consistency, should we update other functions or is there a reason use_elicitation should use concrete types instead of generics?
| ### Supported Schema Types | ||
|
|
||
| MCP elicitation supports string, number, boolean, and enum types with flat object structures only. These limitations ensure reliable cross-client compatibility. See [supported schema types](https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#supported-schema-types) for details. |
There was a problem hiding this comment.
i think this section is valuable since the schema type limitations are key constraint that everybody should know. should we move it earlier or completely remove it?
Uh oh!
There was an error while loading. Please reload this page.